Upgrade to RubyLLM 2.0 - #41

The hook existed to force a second broadcast because "RubyLLM attaches
tool_calls AFTER the parent message is saved". That was true of ruby_llm
1.14.1, the version in the lockfile when the hook landed. It no longer is:
persist_message_completion wraps @message.save! and persist_tool_calls in one
transaction (chat_methods.rb:344-375 in the 1.16.0 now installed), so
Message#after_update_commit already sees the tool-call rows, and
broadcast_replace_later_to re-renders from a fresh reload regardless.

Removed under 1.16 rather than inside the 2.0 commit, so that if the pill did
need a trigger after all it would surface here — where chat.after_message is
still available as a fallback — instead of inside a commit that also renames
two tables. It did not: verified in dev on both tools, create_application and
modify_application, with the pill rendering at tool-call time while the build
was still running.

No replacement mechanism and no new test: the behaviour under test is the
gem's transaction boundary, not app code. The regression net is the existing
pill coverage in messages_helper_test and projects_controller_show_test.

Also drops one UPDATE plus one re-broadcast per tool-call row per turn, and
with it the Chat to Project touch cascade each one triggered.

2.0 is unreleased — it exists only as crmne/ruby_llm@main, which still reports
VERSION 1.16.0 because the bump happens at release. Pinned to c45ebd78 rather
than tracking main: main is explicitly in development and moved 25 commits in
the 27 hours around this work, and BUNDLE_DEPLOYMENT=1 plus HIFUMI_AGENT_IMAGE
reusing this image mean the resolved revision has to be a reviewed decision.
Swap to a version constraint once 2.0 ships to RubyGems.

Everything here is mutually dependent and cannot be split: the acts_as layer
needs the renamed columns, app/models/model.rb raises NoMethodError the moment
it loads without acts_as_model, and use_new_acts_as raises on boot with the new
gem.

Schema. The generated migration renames models to ruby_llm_models and
tool_calls to ruby_llm_tool_calls in place, converts messages.model_id from an
integer FK to provider + model_id strings, moves messages.tool_call_id onto
ruby_llm_tool_calls.result_id, backfills ruby_llm_usages from historical token
columns and then drops them. It is irreversible — no down. Locally: 0.68s,
410 models and 56 tool calls carried over unchanged, 185 usage rows created,
message_type set on every row, no tool-call-id renumbering (this schema already
carried the unique index), integrity_check ok and foreign_key_check empty.
system_injected, thinking_text and thinking_signature survive untouched.

Code. Structured output now returns the JSON String from .content and the Hash
from .parsed, so the two planners and Templates::Picker read .parsed.
RubyLLM::Schema became Schematist::Schema. The tool DSL's params became
parameters. message.tool_calls is now a Hash keyed by provider tool-call id, so
the pill helper takes .values, and the association is ruby_llm_tool_calls,
which the show-page eager load and bin/inspect-chat both needed. Chat's
hand-written with_context override is gone — the gem provides it on the record.

Two containment changes, because v2 removed a defence. v1 parsed structured
output defensively and kept the String on a parse error, which degraded into an
in-band tool error; v2's Message#parsed raises. Unhandled, that would escape
Tool#execute and leave a persisted tool_use with no tool_result — the state
that permanently breaks a chat, and one v2's own orphan cleanup does not cover
since it only runs for RubyLLM/Faraday/Timeout errors. Both tools now rescue
JSON::ParserError alongside InvalidResponse, and Templates::Picker maps it onto
its own InvalidPick. Both are covered by new tests.

Three deviations from the generator's output, all necessary here:

- The migration class is AddRubyLLMV20Columns, not the generated
  AddRubyLlmV20Columns. This app declares inflect.acronym "LLM", so Rails
  camelizes the filename to the former and db:migrate aborts on the latter.
- bin/verify-model-registry reads RubyLLM.config.model_registry_store after
  eager_load! rather than RubyLLM::ActiveRecord::Model directly. The store is
  wired from an on_load :active_record hook, so under this script's standalone
  require the constant and the store are both absent — and the resolver would
  silently fall back to the bundled JSON, the exact confusion the script exists
  to catch.
- PlanSchema requires "schematist" explicitly. The gem requires it only from
  lazily-loaded files, so the constant's existence rode on load order. Nothing
  in the suite resolved PlanSchema — both planner suites stub invoke_llm above
  it — so test/schemas/plan_schema_test.rb now does.

Verified: 565 runs green, rubocop, brakeman, bundler-audit,
bin/verify-model-registry resolving all five offered ids, and a Docker build
that loads the git-sourced gem inside the image (the Dockerfile already strips
bundler/gems/*/.git, which is the path shape a git source creates). In dev, all
four RubyLLM-backed stages exercised end to end: a streamed reply, a build from
chat creating a real Instruction with Revisions, the template stage picking
cyber, a model switch to Opus 5 answering, and a modification plan. Both
migrated and v2-native chats dump clean through bin/inspect-chat.

Four documents described mechanics the upgrade removed, and one of them,
CLAUDE.md, is loaded into every session — leaving it stale actively misleads
future work.

The registry story is rewritten everywhere it appears: RubyLLM owns
ruby_llm_models as its store and falls back to the bundled models.json only
when that table is empty, so the v1 framing about a partially-filled table
shadowing the JSON is gone, along with Model.refresh! in favour of
RubyLLM.models.refresh!. The runbook also now records why the refresh is safe
to run against production without a real OpenRouter key: per-provider fetch
failures are rescued individually and logged "Keeping existing.", and the
store's write is find_or_initialize_by + update! with no deletes. Verified
locally rather than asserted — foreign_key_check stayed empty across a refresh
with 26 live chats.ruby_llm_model_id rows.

Two corrections that came out of running the runbook rather than reading it:

- The row count changed meaning. v1's refresh wrote only what OpenRouter
  discovery returned; v2 fetches the published registry for every provider and
  merges discovery over it, so the store went 410 to 1464 on first refresh. The
  migration preserves the count exactly and a refresh grows it — worth stating,
  since the runbook asks you to compare counts before and after.
- The test environment's bundled-JSON fallback carries sonnet-5 but not
  opus-5, and no test resolves either. Confirmed by resolving both against the
  empty test store.

CLAUDE.md's tool-idempotency bullet cited a guard that no longer exists: the
tool carrying it was deleted in 13e9c1c. It now describes what actually holds
this invariant today — the prompt rule and the BadRequestError banner — and
says plainly that there is no tool-side guard, so the next tool knows it needs
its own. Pre-existing staleness the upgrade merely exposed.

The followups entry proposing a catalog picker "backed by the dormant models
table" is restated: that table is now gem-owned, populated and maintained, so
what remains is capability filtering and a picker, not a table to wake up.

Deliberately untouched: the ~15 SuggestPrompts mentions across the vision,
architecture and phase-2 plan documents. Those are point-in-time records of
what was designed, and the tool's deletion does not make them wrong as history.

CHANGELOG gets an Unreleased entry written for self-hosters, naming the two
things that actually affect them: the git pin, and the one-way migration that
runs automatically at container boot.

…tion

Two points from the PR review.

The picker's local goes back to `content`. Only the accessor changed between
v1 and v2 — the data it holds is the same response content, so the way of
receiving it is no reason to rename the variable. It was also the only such
rename in the branch, which left the picker disagreeing with both sibling
planners, where the local is still `content` and feeds `build_result(content)`.
Swept the rest of the diff to confirm: every other occurrence of `parsed` is
the accessor call itself, a comment naming `Message#parsed`, or the test
double's struct member mirroring the real object's API. Those stay.

The indentation fix covers two lines, not one. The reported test sat at column
0, but the same bad string insertion had also pushed the test after it out to
column 4; both are back at column 2 now.

Worth recording why the linter did not catch it: rubocop-rails-omakase ships
Layout/IndentationWidth and Layout/IndentationConsistency disabled, so a green
`bin/rubocop` says nothing about indentation in this repo. Left alone here
rather than fixed in a RubyLLM upgrade.

Suite still 565 runs green.

The upgrade migration is irreversible and runs itself: bin/docker-entrypoint
runs db:prepare whenever the command is ./bin/rails server, so `kamal deploy`
is what triggers it. There is no manual gate, which makes snapshot-before-deploy
the single most important thing to write down — after the containers boot, the
old schema is gone.

Four sections: rehearse locally on a production snapshot, deploy, verify the
four RubyLLM-backed stages, recover.

Every safety claim in it was verified rather than assumed:

- WAL is really in use (PRAGMA journal_mode returns wal, and both sidecars sit
  beside the database file), so restores stop the process and delete -wal/-shm
  first. Snapshots need no such care: a .backup of a live WAL database produced
  exactly one self-contained file, no sidecars.
- sqlite3 is present at runtime, not just at build time — installed in the
  Dockerfile's base stage, which the runtime stage inherits. Confirmed by
  running sqlite3 --version inside the built image: 3.46.1.
- `kamal app start` starts an existing container while `kamal app boot`
  recreates one (checked against Kamal 2.11.0's own help), which matters in
  rollback: boot re-sources .kamal/secrets from the caller's shell, and
  SMTP_PASSWORD and GITHUB_CLIENT_SECRET are read from the environment there.
  A local export has silently reached production this way before, so the
  recovery section prefers start and gates boot behind a shell check.

Two things the runbook says that a reader would otherwise get wrong: rollback is
code *and* data, because neither image can read the other's schema; and the
ruby_llm_models row count means different things after a migration (preserved
exactly) than after a refresh (grows, 410 to 1464 locally), so a changed count
has to be attributed to whichever step just ran.

Indexed from CLAUDE.md, which closes the two links to this file that the
documentation commit had left dangling.

Covers the RubyLLM 2.0 upgrade: the git pin, the irreversible migration that
renames two tables in place, and the rollout runbook that has to be read before
deploying it.

Minor rather than patch. By the file's own rule this is a judgement call — the
entry adds no functionality a hifumi.dev user can observe, chat and builds
behave exactly as before, which reads as patch. Called minor deliberately: a
major dependency jump onto an unreleased line, plus a one-way migration that
fires automatically on container boot, is not something a self-hoster should
find in a patch release. The weight belongs in the version.

…od race

Four findings from review, three of which reach production.

Malformed-but-valid JSON could still kill a chat. The upgrade widened both
tools' rescues to JSON::ParserError, but .parsed is JSON.parse — valid JSON
that is not an object sails through it and dies one line later, where
Array(content["revisions"]) raises TypeError on an array and NoMethodError on
a boolean. Neither is rescued by #execute, and the gem's own
cleanup_orphaned_tool_results only runs for RubyLLM/Faraday/Timeout errors, so
the tool_use stays persisted with no tool_result and the chat is finished for
good. Guarded in build_result rather than by lengthening the rescue lists:
InvalidResponse is vocabulary both tools already handle, and it makes the three
structured-output call sites agree with Templates::Picker, which has checked
is_a?(Hash) all along. This predates the upgrade; what is new is that the code
claimed to have closed it.

force: true covered only half the race it was added for. FileUtils wraps just
ent.chmod in its rescue — the walk's own Dir.children sits outside it, so a
file vanishing mid-walk was survivable but a directory was not, and git gc
prunes empty .git/objects/<xx> fanouts. Now retried once against a settled
tree. The workspaces this mattered for are every one created before
maintenance.auto/gc.auto started being set at init, which is all of
production's, so those settings are also backfilled on each pass — idempotent,
and two git configs are nothing beside a roast run measured in minutes.

bin/inspect-chat misreported the shape it exists to find: messages[i - 1] with
i = 0 wraps to the last message, so a chat opening with an orphaned tool result
was blamed on the wrong parent instead of reported as having none. The deploy
runbook gates on this script, so it needs to be right.

Templates::Picker's rescue was method-scoped, covering the LLM call as well as
the parse. RubyLLM decodes provider error bodies as JSON too, so an OpenRouter
502 returning an HTML page surfaced as "picker returned malformed JSON" and
pointed whoever read the failed revision at the prompt rather than the
transport. Scoped to .parsed alone.

Each new test was verified by reverting its fix and re-running. That caught one
of them being vacuous: the backfill assertion passed either way, because
test_helper injects the same two settings via GIT_CONFIG_* into every git
subprocess — it now asserts against --local, which ignores the environment.

…e canon

Three small ones from the same review, plus the two that need their own
migration recorded rather than fixed.

bin/verify-model-registry no longer eager-loads the whole app. The guard itself
is still needed — this script boots through require "config/environment" rather
than `bin/rails runner`, and that path really does leave
config.model_registry_store nil, so without it the resolver would silently fall
back to the bundled JSON, which is the confusion the script exists to catch.
But referencing ActiveRecord::Base fires the same on_load hook, so a check
documented as read-only stops loading app/ and lib/ on every run, production
`kamal app exec` included.

Message#tool_calls is an unmemoized Hash the gem rebuilds on every call, one
RubyLLM::ToolCall per row. Two of the three callers only wanted to know whether
any exist, and the gem ships tool_call? for exactly that. On the largest local
chat (53 messages) that is ~106 fewer Hash builds per render. The helper keeps
.values, since it needs the objects.

CLAUDE.md claimed all four RubyLLM tables were "renamed in place". Only two
were; ruby_llm_usages and ruby_llm_batches are created fresh, and the usage
rows are backfilled from columns the migration then drops. Since this file is
loaded into every session and the sentence is about an irreversible migration,
a reader planning a rollback would have gone looking for tables that never
existed. The runbook already had it right.

Recorded in docs/09-ideas/05-followups.md rather than fixed, because the
migration has already run and each needs a new one:

- index_messages_on_provider_and_model_id indexes two columns v2 never writes
  (verified: all 45 messages written since the upgrade have provider NULL,
  against 241 backfilled), on the table that takes a row per streamed message.
  Dropping the columns as well as the index needs a decision first — those
  backfilled values are the only record of which model answered a pre-upgrade
  message.
- The tool_calls to messages foreign key is gone, dropped because the column
  became polymorphic. Harmless while the dependent: :destroy chain is the only
  deletion path, and worth settling before anything ever deletes messages
  outside AR callbacks.

Kept out of the rollout runbook deliberately: neither changes a deploy step,
and the runbook is a procedure, not a data-model changelog.

The is_a?(Hash) guard from 328c7c9 covered the top level only. A valid
JSON object whose `revisions` hold non-objects cleared it and raised one
line later: NoMethodError for String/Integer/nil#fetch, TypeError for
Array#fetch, since Array(hash) reaches the map as [[k, v]]. Neither is
rescued, and neither is caught by RubyLLM's orphan cleanup, which only
fires for RubyLLM/Faraday/Timeout errors. The result is the state
CLAUDE.md calls unrecoverable: a persisted tool_use with no tool_result,
after which every message in that chat is rejected.

Reject non-object elements in both planners, and add a StandardError
backstop at both #execute boundaries. The backstop is the point: a
rescue list cannot be complete here. `revisions.create!` raises
RecordInvalid on a blank summary or prompt, which no shape check in
build_result can prevent.

This inverts the propagation test. Letting an error escape was what
orphaned the tool_use, and ChatRespondJob already rescues StandardError,
so the exception never reached an operator anyway - it only killed the
chat. It is reported via Rails.error instead of swallowed.

The script passed on the exact state it exists to catch. With an empty
ruby_llm_models table every id resolves from the gem's bundled registry,
so it printed "registry store rows: 0" and then "all ids resolve".

But find_or_create_model writes one row per successful resolution, so a
fresh environment self-poisons: the first chat leaves a single row, and
from the next boot that row is the whole registry, because a non-empty
store shadows the bundle completely. That is the 2026-08-12 failure.

Warn when the store is empty, fail when it holds fewer rows than the
offered ids. Runbook 03 said an id had to be "resolvable from one of the
two", which invites the false converse; state that the store wins
whenever it has any rows at all.

Also note that refresh! now holds the write lock for 1464 rows rather
than 410, against the live production database.

Five gaps, each one a step someone following the procedure would get
wrong:

- `kamal app version | tail -1` returns a BLANK line, because kamal
  prints "App Host: <ip>" first and ends with a blank. An empty
  --version is not .presence, so Kamal falls through to the local git
  SHA - during a rollback, the v2 image. The documented rollback booted
  v2 against the restored v1 database and re-ran the one-way migration
  over the snapshot, and the site came back up either way. Take line 2,
  and verify code and data after restoring.

- `kamal deploy` alone leaves the old v1 container serving while the new
  one migrates, against tables just renamed: "no such table:
  tool_calls" on every project page, and with SOLID_QUEUE_IN_PUMA a v1
  job writing dropped columns can orphan a tool_use. Stop the app first,
  after pre-building so the outage is only boot plus migrate.

- A failed deploy stops the NEW container and leaves the old one routed,
  with the migration already committed - the one state neither image can
  read. Say so, and check rather than assuming nothing happened.

- The rehearsal produced no go/no-go: nothing timed the migration
  against kamal's 30s default, and nothing checked that
  backfill_usage_entries carried every row before
  remove_legacy_message_columns dropped the source.

- The secrets warning sat on `kamal app boot`, but `kamal deploy`
  re-sources .kamal/secrets identically, and a prefix check cannot tell
  a stale key from a live one. Compare hashes against production before
  deploying.

Also split the two mutually exclusive rollback commands, since a
block-level copy-paste ran both, and clear jobs enqueued under v2 - the
queue is a separate database that the restore does not touch, and
discard_on DeserializationError is commented out.

visible_in_chat?'s tool_call? disjunct had no test: the one case that
puts a tool call on a message gives it prose too, so content.present?
short-circuits first. The shape it exists for is the empty-content
build-started message, and a regression would silently hide every pill
behind message_row_class's "hidden".

CLAUDE.md cited instructions.txt.erb:18 for the tool-idempotency rule,
which is on 19 - and since the tool-side guard went in 13e9c1c, that
bullet is the only description of the invariant left. Its Phase 2 note
also quoted a pill string that no longer exists and the same deleted
guard.

Note in plan_schema_test.rb that CI's eager_load, not the test, is what
guards the require "schematist": under parallelize any test touching
CreateApplication loads RubyLLM::Tool, which requires schematist for the
rest of the process.

Also record messages.content_raw as a third migration residual, and file
the 2026-08-28 section in date order so it isn't hidden mid-file.